Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Change DialogueView to a mixin class #2652

Merged

Conversation

projectitis
Copy link
Contributor

Description

The DialogueView is responsible for handling and (almost always) displaying the dialog to the user. A very common use case requires the DialogueView to be a PositionComponent or other visual component in the game. This would require extending from both PositionComponent and DialogueView.

Problem: The current DialogueView implementation is an abstract class. Dart does not support multiple inheritance. This means that the case above is difficult to implement.

Solution: Changing the DialogueView from an abstract class to a mixin class allows the developer to extend PositionComponent and mix in DialogueView. The class can now be used as a base class OR as a mixin. All existing tests still pass. An additional test has been added that validates the use as a mixin (extending one class and mixing in DialogueView).

Checklist

  • I have followed the Contributor Guide when preparing my PR.
  • I have updated/added tests for ALL new/updated/fixed functionality.
  • I have updated/added relevant documentation in docs and added dartdoc comments with ///.
  • I have updated/added relevant examples in examples or docs.

Notes on docs: Updated class docs. Checked doc pages. These still read correctly.

Notes on examples: Currently no jenny examples.

Breaking Change?

  • Yes, this PR is a breaking change.
  • No, this PR is not a breaking change.

Related Issues

NA

@projectitis projectitis changed the title Fix: Change jenny's DialogueView yo a mixin class Fix: Change jenny's DialogueView to a mixin class Aug 17, 2023
@projectitis projectitis changed the title Fix: Change jenny's DialogueView to a mixin class fix: change DialogueView to a mixin class Aug 17, 2023
@projectitis projectitis changed the title fix: change DialogueView to a mixin class Fix: change DialogueView to a mixin class Aug 17, 2023
@projectitis projectitis changed the title Fix: change DialogueView to a mixin class fix: Change DialogueView to a mixin class Aug 17, 2023
@spydon spydon requested a review from st-pasha August 18, 2023 08:18
Co-authored-by: Lukas Klingsbo <[email protected]>
@projectitis
Copy link
Contributor Author

Ah, didn't know that was possible, thanks @spydon. Tests still pass, too.

Copy link
Member

@spydon spydon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm, thanks for your contribution! :)

@spydon spydon enabled auto-merge (squash) August 20, 2023 10:24
@spydon spydon merged commit f3d4158 into flame-engine:main Aug 20, 2023
6 checks passed
@projectitis projectitis deleted the fix/jenny-dialogueview-as-mixin-class branch August 26, 2023 21:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants